fix request size limit for multi delete by SylvainSenechal · Pull Request #5868 · scality/cloudserver

SylvainSenechal · 2025-07-17T13:57:53Z

bert-e · 2025-07-17T13:57:57Z

Hello sylvainsenechal,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2025-07-17T13:58:02Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-699 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.1.0

Please check the Fix Version/s of CLDSRV-699, or the target
branch of this pull request.

codecov · 2025-07-17T14:01:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.91%. Comparing base (615f831) to head (c0b1d0c).
⚠️ Report is 437 commits behind head on development/9.1.

Additional details and impacted files

Files with missing lines	Coverage Δ
lib/api/api.js	`86.31% <100.00%> (+0.14%)`	⬆️
lib/api/multiObjectDelete.js	`73.23% <100.00%> (+0.09%)`	⬆️

@@               Coverage Diff                @@
##           development/9.1    #5868   +/-   ##
================================================
  Coverage            75.91%   75.91%           
================================================
  Files                  188      188           
  Lines                11976    11979    +3     
================================================
+ Hits                  9091     9094    +3     
  Misses                2885     2885

Flag	Coverage Δ
ceph-backend-test	`45.46% <100.00%> (+0.01%)`	⬆️
file-ft-tests	`47.34% <100.00%> (+0.01%)`	⬆️
kmip-ft-tests	`27.01% <80.00%> (+0.01%)`	⬆️
mongo-v0-ft-tests	`47.78% <100.00%> (+0.01%)`	⬆️
mongo-v1-ft-tests	`47.80% <100.00%> (+0.01%)`	⬆️
multiple-backend	`34.00% <80.00%> (+0.01%)`	⬆️
sur-tests	`34.62% <100.00%> (-0.02%)`	⬇️
sur-tests-inflights	`36.57% <100.00%> (+0.01%)`	⬆️
unit	`66.94% <100.00%> (+<0.01%)`	⬆️
utapi-v2-tests	`33.35% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bert-e · 2025-07-17T14:07:50Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

ghost · 2025-07-17T14:29:30Z

+                const defaultMaxPostLength = request.method === 'POST' ?
+                      1024 * 1024 : 512 * 1024; // 1 MB or 512 KB


as an optim I think these values should be computed outside of the function, as it would be evaluated in every request otherwise, let's compute it on top of the file?

it depends on request.method, so can't really be computed earlier... And apisLengthLimits is really a constant, so not sure it will change much...

I mean the 1024*1024 and 512*1024, this can be the real value directly.

I can set it in the constants.js file

ghost · 2025-07-17T14:31:28Z

-                    if (err || objects.length < 1 || objects.length > 1000) {
-                        return next(errors.MalformedXML);
+                    if (err || objects.length < 1 || objects.length > constants.maximumBatchDeleteObjects) {
+                        return next(errors.MalformedXML.customizeDescription(


To use customizeDescription we must not use errors but errorInstances. I think the error should stay the same, unless we are able to confirm what AWS exactly sends, we should be 100% consistent, so we can just keep the base error IMHO

I thought it would be nice to have a more descriptive errors but yeah if we want to stick the same aws errors I'll remove it

ghost · 2025-07-17T14:33:39Z

-                const MAX_POST_LENGTH = request.method === 'POST' ?
-                      1024 * 1024 : 1024 * 1024 / 2; // 1 MB or 512 KB
+                const apisLengthLimits = {
+                    // Multi Objects Delete request can be large : up to 1000 keys of 1024 bytes is


Were you able to confirm we reach the limit with the 1000 keys?

probably depends on the key length : which can be up to 1024 bytes per key, according to AWS...

Yeah I called the api with smaller requests, and change manually the max_post_length to a significantly lower value to trigger my errors. I logged the body length too and yeah with 1000 long keys it's gonna go above 1MB

francoisferrand · 2025-07-17T14:47:44Z

-                        return next(errors.MalformedXML);
+                    if (err || objects.length < 1 || objects.length > constants.maximumBatchDeleteObjects) {
+                        return next(errors.MalformedXML.customizeDescription(
+                            `The provided request tries to delete ${objects.length} objects, ` +


is this the message that AWS returns? (the AWS standard part looks weird)

as a general rule of thumb, we want to stick to same behavior (esp errors) as AWS

No it's my own error, I realized we want to have the exact same errors as aws so I'll remove it

Issue : CLDSRV-699

SylvainSenechal · 2025-07-22T11:48:54Z

/approve

bert-e · 2025-07-22T11:49:06Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/9.1

The following branches will NOT be impacted:

development/7.10
development/7.4
development/7.70
development/8.8
development/9.0

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2025-07-22T12:39:54Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/9.1

The following branches have NOT changed:

development/7.10
development/7.4
development/7.70
development/8.8
development/9.0

Please check the status of the associated issue CLDSRV-699.

Goodbye sylvainsenechal.

SylvainSenechal force-pushed the bug/CLDSRV-699 branch from f7881d5 to eb64cfc Compare July 17, 2025 14:00

SylvainSenechal force-pushed the bug/CLDSRV-699 branch from eb64cfc to 9c4d893 Compare July 17, 2025 14:07

ghost reviewed Jul 17, 2025

View reviewed changes

francoisferrand reviewed Jul 17, 2025

View reviewed changes

SylvainSenechal force-pushed the bug/CLDSRV-699 branch from 9c4d893 to 634e1ba Compare July 17, 2025 15:37

francoisferrand reviewed Jul 21, 2025

View reviewed changes

Comment thread constants.js Outdated

francoisferrand reviewed Jul 21, 2025

View reviewed changes

Comment thread lib/api/multiObjectDelete.js Outdated

francoisferrand approved these changes Jul 21, 2025

View reviewed changes

changed request size limit for multi delete objects

c0b1d0c

Issue : CLDSRV-699

SylvainSenechal force-pushed the bug/CLDSRV-699 branch from 634e1ba to c0b1d0c Compare July 22, 2025 09:27

ghost approved these changes Jul 22, 2025

View reviewed changes

bert-e merged commit cd4b15e into development/9.1 Jul 22, 2025
26 checks passed

bert-e deleted the bug/CLDSRV-699 branch July 22, 2025 12:40

		const defaultMaxPostLength = request.method === 'POST' ?
		1024 * 1024 : 512 * 1024; // 1 MB or 512 KB

Conversation

SylvainSenechal commented Jul 17, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bert-e commented Jul 17, 2025

Hello sylvainsenechal,

Uh oh!

bert-e commented Jul 17, 2025

Incorrect fix version

Uh oh!

codecov bot commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bert-e commented Jul 17, 2025

Waiting for approval

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Jul 17, 2025 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SylvainSenechal commented Jul 22, 2025

Uh oh!

bert-e commented Jul 22, 2025

In the queue

Uh oh!

bert-e commented Jul 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SylvainSenechal commented Jul 17, 2025 •

edited by atlassian bot

Loading

codecov bot commented Jul 17, 2025 •

edited

Loading

ghost Jul 17, 2025 •

edited by ghost

Loading